Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Unskip install_large_prebuilt_rules_package #192563

Merged

Conversation

maximpn
Copy link
Contributor

@maximpn maximpn commented Sep 11, 2024

Addresses: #192479

Summary

This PR unskips install_large_prebuilt_rules_package FTR test.

Details

The flakiness is caused by chunked assets installation inside the Fleet plugin. Used no index refresh strategy leads to reading stale data in PUT /api/detection_engine/rules/prepackaged endpoint handler and returning arbitrary data.

The problem was fixed by adding refresh=wait_for for the last assets chunk installation.

Flaky test runner

@maximpn maximpn added release_note:skip Skip the PR/issue when compiling release notes test-failure-flaky Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.16.0 labels Sep 11, 2024
@maximpn maximpn self-assigned this Sep 11, 2024
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6916

[❌] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/large_prebuilt_rules_package/trial_license_complete_tier/configs/ess.config.ts: 62/100 tests passed.

see run history

@maximpn maximpn force-pushed the unskip-install_large_prebuilt_rules_package branch 2 times, most recently from 5410688 to 716ba52 Compare September 13, 2024 08:41
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6933

[❌] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/large_prebuilt_rules_package/trial_license_complete_tier/configs/ess.config.ts: 66/100 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6935

[❌] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/large_prebuilt_rules_package/trial_license_complete_tier/configs/ess.config.ts: 0/100 tests passed.

see run history

@maximpn maximpn force-pushed the unskip-install_large_prebuilt_rules_package branch from 716ba52 to d8f27d2 Compare September 13, 2024 09:59
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6936

[❌] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/large_prebuilt_rules_package/trial_license_complete_tier/configs/ess.config.ts: 57/100 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6937

[❌] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/large_prebuilt_rules_package/trial_license_complete_tier/configs/ess.config.ts: 0/100 tests passed.

see run history

@maximpn maximpn force-pushed the unskip-install_large_prebuilt_rules_package branch from d8f27d2 to a22e630 Compare September 13, 2024 10:43
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6938

[✅] x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/prebuilt_rules/large_prebuilt_rules_package/trial_license_complete_tier/configs/ess.config.ts: 100/100 tests passed.

see run history

@maximpn maximpn added the fixed label Sep 13, 2024
@maximpn maximpn marked this pull request as ready for review September 13, 2024 12:29
@maximpn maximpn requested review from a team as code owners September 13, 2024 12:29
@maximpn maximpn requested a review from dplumlee September 13, 2024 12:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@maximpn maximpn requested review from jpdjere and removed request for dplumlee September 13, 2024 12:31
Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀 Do you have an idea of the performance impact that refresh: wait_for has? I guess it will slow down the install but curious to see if it's measurable

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Sep 13, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@maximpn maximpn changed the title [Security Solution][DO NOT MERGE] Unskip install_large_prebuilt_rules_package [Security Solution] Unskip install_large_prebuilt_rules_package Sep 13, 2024
@maximpn
Copy link
Contributor Author

maximpn commented Sep 15, 2024

@nchaulet

Do you have an idea of the performance impact that refresh: wait_for has? I guess it will slow down the install but curious to see if it's measurable

installKibanaAssets is invoked once per package installation (correct me if I'm wrong here) and in the worst case it has to wait for index auto-refresh (it happens every 1s in ESS and 5s in Serverless). It might be noticeable on not-loaded systems while installing tiny packages. I compared execution time of the test I unskipped before and after the change. And I haven't noticed any differences it takes ~50s.

Let me know if you think we need to test refresh: wait_for more thoroughly.

Copy link
Contributor

@jpdjere jpdjere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @maximpn

@maximpn maximpn merged commit b3baadd into elastic:main Sep 16, 2024
21 checks passed
@maximpn maximpn deleted the unskip-install_large_prebuilt_rules_package branch September 16, 2024 20:57
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 16, 2024
…astic#192563)

**Addresses:** elastic#192479

## Summary

This PR unskips `install_large_prebuilt_rules_package` FTR test.

## Details

The flakiness is caused by chunked assets installation inside the Fleet plugin. Used no index refresh strategy leads to reading stale data in `PUT /api/detection_engine/rules/prepackaged` endpoint handler and returning arbitrary data.

The problem was fixed by adding `refresh=wait_for` for the last assets chunk installation.

## Flaky test runner

- 🟢  (100 runs) https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6938

(cherry picked from commit b3baadd)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 16, 2024
…ackage` (#192563) (#193091)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Unskip
`install_large_prebuilt_rules_package`
(#192563)](#192563)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maxim
Palenov","email":"maxim.palenov@elastic.co"},"sourceCommit":{"committedDate":"2024-09-16T20:57:22Z","message":"[Security
Solution] Unskip `install_large_prebuilt_rules_package`
(#192563)\n\n**Addresses:**
https://github.com/elastic/kibana/issues/192479\r\n\r\n##
Summary\r\n\r\nThis PR unskips `install_large_prebuilt_rules_package`
FTR test.\r\n\r\n## Details\r\n\r\nThe flakiness is caused by chunked
assets installation inside the Fleet plugin. Used no index refresh
strategy leads to reading stale data in `PUT
/api/detection_engine/rules/prepackaged` endpoint handler and returning
arbitrary data.\r\n\r\nThe problem was fixed by adding
`refresh=wait_for` for the last assets chunk installation.\r\n\r\n##
Flaky test runner\r\n\r\n- 🟢 (100 runs)
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6938","sha":"b3baadd14ae0cc07e22d878bdd24861309a26426","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","test-failure-flaky","Team:Fleet","v9.0.0","Team:Detections
and Resp","fixed","Team: SecuritySolution","Team:Detection Rule
Management","v8.16.0"],"title":"[Security Solution] Unskip
`install_large_prebuilt_rules_package`","number":192563,"url":"https://github.com/elastic/kibana/pull/192563","mergeCommit":{"message":"[Security
Solution] Unskip `install_large_prebuilt_rules_package`
(#192563)\n\n**Addresses:**
https://github.com/elastic/kibana/issues/192479\r\n\r\n##
Summary\r\n\r\nThis PR unskips `install_large_prebuilt_rules_package`
FTR test.\r\n\r\n## Details\r\n\r\nThe flakiness is caused by chunked
assets installation inside the Fleet plugin. Used no index refresh
strategy leads to reading stale data in `PUT
/api/detection_engine/rules/prepackaged` endpoint handler and returning
arbitrary data.\r\n\r\nThe problem was fixed by adding
`refresh=wait_for` for the last assets chunk installation.\r\n\r\n##
Flaky test runner\r\n\r\n- 🟢 (100 runs)
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6938","sha":"b3baadd14ae0cc07e22d878bdd24861309a26426"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192563","number":192563,"mergeCommit":{"message":"[Security
Solution] Unskip `install_large_prebuilt_rules_package`
(#192563)\n\n**Addresses:**
https://github.com/elastic/kibana/issues/192479\r\n\r\n##
Summary\r\n\r\nThis PR unskips `install_large_prebuilt_rules_package`
FTR test.\r\n\r\n## Details\r\n\r\nThe flakiness is caused by chunked
assets installation inside the Fleet plugin. Used no index refresh
strategy leads to reading stale data in `PUT
/api/detection_engine/rules/prepackaged` endpoint handler and returning
arbitrary data.\r\n\r\nThe problem was fixed by adding
`refresh=wait_for` for the last assets chunk installation.\r\n\r\n##
Flaky test runner\r\n\r\n- 🟢 (100 runs)
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6938","sha":"b3baadd14ae0cc07e22d878bdd24861309a26426"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Maxim Palenov <maxim.palenov@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team:Fleet Team label for Observability Data Collection Fleet team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. test-failure-flaky v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants